-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port infrastructure to pyproject.toml #29
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a couple of comments related to the dependencies that needs addressing, but otherwise this looks good.
The documentation fails to build with mkdocs serve
, but it was also failing when using the original setup.py
version, so it is not an issue introduced in this PR. I'll open an issue so this can be addressed in the future.
pyproject.toml
Outdated
{ name = "Imperial College London RSE Team", email = "[email protected]" }, | ||
{ name = "Barnaby Dobson", email = "[email protected]" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe reverse the order of these. After all, Barney was the initiator...
{ name = "Imperial College London RSE Team", email = "ict-rse-team@imperial.ac.uk" }, | |
{ name = "Barnaby Dobson", email = "b.dobson@imperial.ac.uk" } | |
{ name = "Barnaby Dobson", email = "b.dobson@imperial.ac.uk" }, | |
{ name = "Imperial College London RSE Team", email = "ict-rse-team@imperial.ac.uk" } |
pyproject.toml
Outdated
] | ||
|
||
demos = [ | ||
"pandas", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been trying to run all the tutorials and for them to work we need to pin Pandas to 1.5.2 since they are using some features that have been removed in the current version (2.1.2). In particular, I get:
File "C:\Users\dalonsoa\Projects\wsi\docs\demo\scripts\oxford_demo.py", line 120, in <module>
dates.sort()
^^^^^^^^^^
AttributeError: 'DatetimeArray' object has no attribute 'sort'
So we need this:
"pandas", | |
"pandas==1.5.2", |
And then recreate the requirements-demos.txt
file. Also open an issue highlighting this, so the use of deprecated features can be fixed in the future (possibly by us).
"shapely" | ||
] | ||
|
||
doc = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that when requiring to install the documentation requirements you should install both the demo
and doc
requirements. See
Line 11 in 741500a
docs_requires = f.read().splitlines() + demos_requires |
I'm not totally sure how to make this work here for a tool that is not in pypi
(yet). Maybe try:
doc = [
"mkdocs",
...
"wsimod[demos]"
]
This is OK for pypi
hosted tools, but I'm not totally sure this will work for wsimod
yet. Maybe ".[demos]"
would do the trick?
If this is changed, the requirements-doc.txt
file will need to be recreated.
This is the issue where I point to the building documentation problem: #30 |
I've made the requested changes. Also changed the project name to lowercase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me! Well done!
Switched from setup.py to pyproject.toml. Also generated new requirements files, and updated the README accordingly.
In the process, I spotted the requirement 'dill' which I have added to pyproject.toml and the README
Closes #21